Skip to content

NimBLE peripheral shim: run firmware BLE code in the simulator - #36

Open
rfordinal wants to merge 13 commits into
crosspoint-reader:mainfrom
rfordinal:feat/nimble-peripheral-shim
Open

NimBLE peripheral shim: run firmware BLE code in the simulator#36
rfordinal wants to merge 13 commits into
crosspoint-reader:mainfrom
rfordinal:feat/nimble-peripheral-shim

Conversation

@rfordinal

@rfordinal rfordinal commented Aug 23, 2026

Copy link
Copy Markdown

PR 2 of 2 — A NimBLE peripheral shim, so firmware BLE code runs in the simulator

This is the second of two, and it depends on #35 (the FreeRTOS shim). This
branch alone has no xSemaphoreCreateBinary and no pdMS_TO_TICKS, so a
firmware BLE file does not build against it until that one lands. Merging this
first will look broken and will not be.

Where this comes from

We maintain ExplorInk, a fork of
CrossPoint Reader that turns an Xteink X3/X4 into an offline map and navigation
device, and our fork of this simulator is
explorink-simulator. The
firmware referred to below as "the firmware this was developed against" is that
one, and it is public, so every claim here is checkable rather than asserted.

The code and docs in this PR name none of that on purpose. Anything you
merge you then have to maintain, and a downstream project's file paths and build
flags in your tree would be noise you did not sign up for. So the diff is
generic, the citations point at NimBLE and FreeRTOS rather than at us, and the
provenance lives here in the description where it costs you nothing.

What it is

The simulator gains a shim for the Bluetooth radio: a header-compatible
fake, meaning the same C++ API as NimBLE-Arduino with a different implementation
underneath. No NimBLE source is compiled. A client connects to a TCP socket
on loopback, speaks newline-delimited JSON, and plays the part of the central.

So firmware BLE code runs unchanged on the host. Everything above the radio is
the firmware's own; only the radio is faked.

Off unless asked: CROSSPOINT_SIM_BLE_PORT=8765. Absent or 0 and the simulator
behaves exactly as it does today, no listener, no thread.

Why it might interest you rather than just us

Anything in a firmware that talks over BLE is currently untestable in the
simulator, and the shim makes the whole path executable on a laptop: a command
channel, a file push, a request-and-fetch loop. It also buys fault injection
that hardware cannot: withhold an indication confirm, drop the link mid-transfer,
send a malformed frame, subscribe to nothing and then transfer.

The four things it has to get right, or it lies

A polite fake is worse than none, because it hides exactly the bugs the real
stack produces. These are reproduced deliberately:

  1. Callbacks run on a dedicated host thread, never inline on the caller's.
    Inline dispatch makes a whole class of deadlock unreproducible.
  2. An indication confirm is out of band and withholdable. indicate()
    returns when the pending slot accepted the payload, not when the peer got it;
    the confirm arrives later through onStatus.
  3. A second indicate() before a confirm clobbers the first. Measured on
    real hardware: back-to-back calls all return true and the peer sees the first
    and the last. The shim reproduces the clobber and emits a clobber event so
    it is observable instead of silent.
  4. The client sets the MTU. MTU drives a firmware's payload arithmetic, so a
    wrong default tests different arithmetic than a device runs.

One bug worth reading about, because it is the failure mode of this whole idea

indicate() originally returned false when nobody was subscribed, when
nothing was connected, and when the characteristic lacked NOTIFY/INDICATE, under
a comment reading "refusals the real stack makes".

Real NimBLE makes none of those three checks. NimBLECharacteristic::sendValue
has no CCCD check, no property check and no connection check; with the default
connHandle it falls into the peer loop, and an empty peer list leaves rc = 0
and returns true. The value came from a comment in the firmware this was
developed against, which is itself wrong, rather than from the library on disk.

The cost, measured with a connected-but-unsubscribed peer: the shim drove the
firmware's 40 x 25 ms retry loop, about 1 s per line, and never set the
firmware's confirm-timed-out flag. Real hardware takes the 3000 ms confirm wait
and does set it. Different duration, different branch, different persistent
state.

No test could have caught it: two self-test checks asserted the inversion as
correct, so the suite agreed with the bug. A test written from the same belief as
the code confirms the belief, not the behaviour. It is fixed, the checks are
inverted rather than deleted, and the story is in docs/ble-shim.md because it
is the most useful thing in there for the next person.

Verification

  • 50 GATT self-test checks, 0 failures, clean under ThreadSanitizer. Drives
    connect, subscribe, write, indicate, confirm, clobber, disconnect against a
    fake sink, and asserts a callback ran on a different thread than the caller.
  • 9 attach checks over a real loopback socket: a client attaching after the
    table is built receives the current state unasked.
  • 65 transport checks, plus ASan/UBSan and TSan. Every client op with
    explicit and defaulted fields, a second client refused, 20 malformed inputs,
    an over-long line, a prompt stop() with a client connected, and concurrent
    emit() from two threads arriving unspliced.
  • A firmware translation unit compiles against it, which is what proves the
    API surface complete rather than plausible.
  • Driven end to end by a real phone. A separate host process (not in this
    PR, it lives downstream) bridges a real BlueZ GATT peripheral to this socket,
    so an Android app connected over actual Bluetooth to firmware running as a host
    binary: it negotiated MTU 517, subscribed, and its GPS fix reached the firmware
    and redrew the map. That is what convinced us the wire contract is right.

Notes for review, including two things you may not want

  • Two new top-level directories, docs/ and tests/. tests/ is the one to
    question: CLAUDE.md says this repo has no tests and that a change is tested
    by running the simulator, so this introduces a convention rather than following
    one. We have kept the tests because they are how the bug above was caught and
    how the fix is pinned, but we are not attached to the layout. Say the word and
    they move, merge into the sample runner, or go.
  • No JSON library is added. The parse is hand-rolled for the ten op shapes,
    because a dependency for this would be a poor trade.
  • Loopback only, INADDR_LOOPBACK, never INADDR_ANY. The process runs
    firmware command handling, so exposing it on a LAN interface would be a real
    hazard rather than a style point.
  • What it cannot answer is written down in docs/ble-shim.md rather than
    left to be discovered: heap (the simulator reports a flat figure, and a real
    BT host is usually the biggest single RAM consumer), the radio itself, and a
    peer's real GATT stack. A python client agreeing with a firmware proves the
    firmware self-consistent, not interoperable.
  • One property value was wrong and is fixed here: NIMBLE_PROPERTY::READ was
    0x0001, which is broadcast's bit. Read is 0x0002 per host/ble_gatt.h.
    Nothing observable rode on it, and it would have misled the first reader who
    made a characteristic readable.
  • Nothing ran on real e-ink hardware. We have none on hand at the moment.

Roman Fordinal and others added 13 commits August 23, 2026 14:42
SimBleLink.h is the interface between the socket transport and the GATT model.
Both halves are built in parallel against it, so it is frozen before either
starts. docs/ble-shim.md seeds the wire protocol, the threading model and the
four fidelity items the shim has to reproduce.
SimBleLink.cpp is the socket half of the frozen seam: a loopback TCP
listener, one reader thread that lives in poll(), line framing over
recv boundaries, and a thread safe emit(). SimBleProtocol.{h,cpp} is
the hand rolled JSON codec for the ten client ops. No JSON library is
added: the ops are flat and this branch must not grow a dependency.

Transport decisions worth naming:

- The listener binds 127.0.0.1, never INADDR_ANY. This process runs
  firmware command handling.
- stop() wakes the reader with a self pipe byte. shutdown() on a
  listening socket does not portably wake accept(), and closing a fd
  another thread is polling races a new socket onto that number.
- One line caps at 65536 bytes. Past it the buffer is dropped, one
  error event goes out, and bytes up to the next newline are discarded.
- The reader alone closes the client fd. emit() shuts it down on a
  write failure so teardown stays in one place, and carries a 5 s send
  timeout so a wedged client cannot hang a firmware thread.
- A lost socket synthesizes a disconnect op, so the GATT model does not
  keep believing a central is connected.

tests/sim_ble_link_selftest.{cpp,py} is the gate. It builds against
those two sources alone, so the transport is provable before the GATT
model exists: 65 checks covering every op explicit and defaulted, 20
malformed lines, the cap, the second client refusal, prompt stop() and
concurrent emit. Clean under ASan+UBSan and under TSan.
Adds a "Transport specifics" section with the settled numbers: the
65536 byte line cap and what a longer line does, how stop() wakes a
blocked reader and why the self pipe beats the alternatives, the
loopback only bind and its reason, the second client refusal, the
malformed line answer, emit()'s line atomicity, the synthetic
disconnect on socket loss, and the full default and range table for
every op field.

Flips those claims to [verified] and names the command that showed
them. The GATT half stays [contract]. Every claim carries a file:line.
Header-compatible fake for NimBLE-Arduino's peripheral API, so the firmware's
BLE code runs on the host. No NimBLE source is compiled.

The NimBLE* classes forward; SimBleGatt holds the model, the host thread, the
single per-connection indication slot and the outgoing event JSON. Callbacks
are dispatched on the host thread, never inline on the caller's, because the
firmware is built around the disconnect callback and the sync event it waits
for sharing one task.

Three fidelity points the shim reproduces rather than smooths over: the
indication confirm is out of band and can be withheld, a second indicate()
before a confirm clobbers the first and still returns true, and the client owns
the MTU (default 23).

Self-test: 40 checks, 0 failures, clean under ThreadSanitizer. The real
firmware translation unit compiles against these headers.
Flips the threading model, the four fidelity items and the enforced refusals
from contract to verified, each naming the self-test check that showed it.

Adds what building it turned up: two NimBLE calls the frozen contract list
missed (NimBLEServer::start and setCallbacks' second argument), the indication
slot being per connection rather than per characteristic, and two FreeRTOS
symbols the simulator still lacks that the firmware BLE file needs.

Extends "What it cannot answer" with the one refusal this shim cannot produce:
indicate() never returns false for a busy slot, so the firmware's park-and-flush
path is not exercised here.
The self-test defined a second main() and a second SimBleLink under src/.
library.json has no srcFilter, so both landed in the library archive next to
simulator_main.o and the linker satisfied main() from the test: the simulator
built clean, never ran, printed one line from a global constructor and aborted
in a destructor of code it had never entered.

Moved to tests/, which is where the transport self-test already lives and which
is why only this half broke. No srcFilter: an allowlist has to be remembered
when the next file appears, a directory that is not compiled does not.

The test now ends with fflush(stdout) and _exit() instead of returning, so its
verdict cannot be lost to someone else's teardown.
NimBLEDevice::init() emits stack/up, and init() is also what starts the
listener, so no client can be attached when that line is written and it is
dropped. The event was never racy, it was structurally undeliverable. The gatt
table and the first advertising line have the same problem whenever the firmware
builds them before a client turns up, which is the normal case. A real central
does not have this problem: it scans.

The accept path now synthesizes an `attach` op into the sink, the mirror of the
synthetic disconnect a lost socket already produces, and the GATT model answers
it by emitting what is currently true: stack up, the gatt table if one is built,
then the current advertising state. Not a real BLE event, same status as
clobber. No change to the frozen SimBleLink.h: the op name is a string.

Nothing is replayed about a live connection. A client that was not there for the
connect is not the central that made it.

The transport gate's two connect-time baselines were snapshotted before the
reader had accepted, so they measured the next op against the attach line and
failed intermittently. They now wait for the attach op and assert it arrives,
which adds a check rather than removing one. 65/65 again, plain, ASan/UBSan and
TSan.

tests/sim_ble_gatt_attach_selftest.cpp proves the replay over a real socket: it
builds a table with nobody connected, connects to itself and reads the three
lines. 9 checks.
Records the state replay as its own section, labelled not-a-real-BLE-event the
same way clobber is, so a client author reading a packet trace does not mistake
it for something a phone would see. Says what is not replayed and what a client
that attaches before init() gets.

Rewrites the self-test commands for the new tests/ paths and adds the socket
test. States why test files live in tests/ rather than behind a srcFilter.

Re-checks every file:line citation in the file. The accept-path insertion moved
21 lines of src/SimBleLink.cpp, which stale-dated eleven citations in the
transport section, and the GATT edits moved eighteen of mine. One citation was
already off by one before this pass.
The bullet claiming the simulator lacked pdMS_TO_TICKS and
xSemaphoreCreateBinary, and that the firmware BLE file therefore did not build
in-tree, is false on all three counts now. Deleted rather than softened. The
firmware side documents its own fix; a second account of it here would drift.

The same claim was embedded in the firmware syntax-check recipe as a
placeholder -include line. Removed. The recipe now says only what it is for:
anything the -include flags miss surfaces as a FreeRTOS name, not a NimBLE one.
The stronger evidence replaces it: the whole firmware builds against the shim
with no NimBLE symbol missing.

Three more stale facts found by auditing content rather than line numbers:

- The status line still called the file a seed written before the
  implementation, contradicting its own next sentence. Both halves are built.
- Fault injection was marked all contract. Withholding a confirm and sending a
  malformed frame are both demonstrated now; the other two need a transfer, so
  they need the firmware.
- The busy-slot bullet listed its refusals as if exhaustive and omitted the
  stack being down.

The heap, radio, peer-GATT-stack and busy-slot bullets are unchanged in
substance. All 54 citations still resolve.
The shim's own doc was written from its self-tests. Real firmware has now
been driven through it: pin the gatt event's props type on the wire (the one
field nobody typed, and it broke every client), record what the socket
structurally cannot answer (throughput, MTU negotiation, an over-MTU
refusal, radio loss, a hole from a withheld confirm, storage failures),
note that auto_confirm survives a client disconnect, add speed as a fifth
fidelity hazard in both directions, and promote the four fault-injection
cases that needed firmware from contract to verified.

Also: a grep over call syntax cannot detect a stale API contract, and six
firmware citations had drifted.
NIMBLE_PROPERTY::READ was 0x0001. That is BLE_GATT_CHR_F_BROADCAST. Read is
0x0002 (nimble/host/include/host/ble_gatt.h:133), and standard GATT agrees, so
it was not a NimBLE quirk being matched.

Harmless today and wrong for upstream. No firmware characteristic sets READ, so
nothing observable rode on it and no test could have caught it. But the property
bitmask now goes out on the wire as an integer and a client decodes it back into
names, so a readable characteristic would have made the shim say "broadcast"
where the device meant "read".

WRITE, NOTIFY and INDICATE were already right: 0x0008, 0x0010 and 0x0020, at
ble_gatt.h:139, :142 and :145. Each constant now carries its source line, and
the block says why 0x0001 is not read. WRITE_NO_RSP and the four flags above
INDICATE stay undeclared: the firmware does not use them.

The doc's props decoding table said `1` read, propagated from the same mistake.
Corrected, and it now names the header to check rather than asking the reader to
trust the table.
The shim returned false from indicate() when nothing was connected, when nobody
was subscribed, and when the characteristic lacked NOTIFY/INDICATE, under a
comment claiming those were refusals the real stack makes. It makes none of
them.

The firmware calls the two-argument indicate(), so connHandle defaults to
BLE_HS_CONN_HANDLE_NONE (NimBLECharacteristic.h:60) and the call lands in
sendValue (NimBLECharacteristic.cpp:272-328). That function has no connection
check, no CCCD check and no property check: rc starts at 0, only the peer loop
can move it, and with no peers the loop body never runs. It returns true.
ble_gatts_indicate_custom refuses only on out of memory or, under
BLE_GATT_CACHING, an unaware peer (ble_gattc.c:4888-4936), so an unsubscribed
peer gets a real indication PDU and the pending slot is genuinely taken.

Measured against the real firmware with a connected-but-unsubscribed peer:

  false: reply indicate failed after 40 attempts   ~1008 ms, flag never set
  true:  reply unconfirmed after 3000 ms            3000 ms, flag set

Different duration, different branch, different log line, and different
persistent state: only the second sets lastConfirmTimedOut_, which is what
suppresses FETCH_CANCEL downstream. The simulator was sending developers into
code the device never reaches.

Nothing connected returns true and leaves the slot alone, emitting nothing:
real NimBLE builds and sends nothing there, and filling the slot would let a
later connect-then-confirm confirm a payload from before the link existed. A
connected but unsubscribed peer takes the slot and gets the indicate event,
because the PDU really does go out -- but no auto-confirm is scheduled, since a
client with the CCCD off has nothing to confirm. That is what reproduces the
firmware's confirm timeout.

Two more corrections found in the same read. An empty payload is a different
operation in real NimBLE, not a refusal: it falls through to
ble_gatts_chr_updated and still returns true. The stack-down and null-pointer
guards stay false and are now labelled as shim guards, because real NimBLE
dereferences a null server there rather than returning anything.

Clobber semantics are untouched: a second indicate() before a confirm still
overwrites and still returns true.

The two self-test checks that asserted the inversion are inverted rather than
deleted, with the consequence added: unsubscribed indicate is true, the event
still goes out, and nothing confirms it. The disconnect check that leaned on
the old return value now proves the subscription cleared by its consequence --
no confirm on the new link, and a confirm again after re-subscribing. 50 checks.
Nineteen comment and doc references pointed at BlePositionServer.cpp and
FREEINK_CAP_BLE_PERIPHERAL -- a file and a build flag that do not exist upstream,
which made the branch unreviewable by anyone outside this fork.

Each one now states the behaviour instead of the address. Nothing load-bearing
was lost: the authority in these comments was always NimBLE's own source
(NimBLECharacteristic.cpp, ble_gattc.c, ble_gatt.h), which an upstream reader can
check, and the firmware was only ever provenance. The syntax-check recipe keeps
its shape with placeholders rather than one fork's paths.

Gates unchanged: 50 GATT checks, 9 attach checks, 65 transport checks.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@rfordinal, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 401dffec-c862-4218-b305-70db57097c4f

📥 Commits

Reviewing files that changed from the base of the PR and between 8323320 and 3ff80e0.

📒 Files selected for processing (19)
  • docs/ble-shim.md
  • src/NimBLEAttValue.h
  • src/NimBLECharacteristic.h
  • src/NimBLEConnInfo.h
  • src/NimBLEDevice.cpp
  • src/NimBLEDevice.h
  • src/SimBleGatt.cpp
  • src/SimBleGatt.h
  • src/SimBleLink.cpp
  • src/SimBleLink.h
  • src/SimBleProtocol.cpp
  • src/SimBleProtocol.h
  • src/host/ble_gap.h
  • tests/sim_ble_gatt_attach_selftest.cpp
  • tests/sim_ble_gatt_selftest.cpp
  • tests/sim_ble_gatt_selftest.h
  • tests/sim_ble_gatt_stub.cpp
  • tests/sim_ble_link_selftest.cpp
  • tests/sim_ble_link_selftest.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant